Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/Cucumber Messages #233

Open
wants to merge 224 commits into
base: main
Choose a base branch
from
Open

Feature/Cucumber Messages #233

wants to merge 224 commits into from

Conversation

clrudolphi
Copy link
Contributor

Draft PR to provide a space for reviews and suggestions on this Feature.

🤔 What's changed?

Adds support for creation of Cucumber Messages during the execution of Reqnroll scenarios.

Changes are in three main area:
FeatureGeneration: as a feature file is processed and used to generate C# code, the Gherkin toolchain is used to create the first few Messages for the feature. These are attached to the FeatureInfo object.
CucumberMessagePublication: written as an embedded plugin, this component listens for ExecutionEvents and converts them, and their attached state, to Cucumber Messages. These are 'published' to one or more sinks.
FileSinkPlugin: a traditional runtime plugin, this plugin registers with the Publishing component to receive the Messages as they are released by the Publisher and stored to a file (one per feature file).

Messages are strings serialized from the CucumberMessage Envelope format and stored one per line in an ".ndjson" result file.

⚡️ What's your motivation?

To provide future compatibility with the broader Cucumber eco-system of tooling.
To provide the input to a future implementation of a Reqnroll replacement to LivingDocs.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

Anything.

📋 Checklist:

  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

@clrudolphi
Copy link
Contributor Author

FYI, github Build is failing b/c one produced component (FileSinkPlugin) gets published only to my local Nuget store. It can then not be found by the build pipeline in github.

@clrudolphi clrudolphi added the enhancement New feature or request label Aug 17, 2024
@clrudolphi
Copy link
Contributor Author

clrudolphi commented Aug 19, 2024

First end-to-end working scenario is operational.
Needed out of a PR review:

  • Overall code structure
  • Patterns that have been used (good/bad) and patterns that should have been used
  • Thread-safety: is the level of thread awareness and approach to thread-safety appropriate and adequate?

Next:

  • Copy the Cucumber Compatibility Kit scenarios into an integration test suite and build out the structure to build, invoke, and confirm compliance.

@clrudolphi
Copy link
Contributor Author

Build is failing b/c I have a private build of Cucumber Messages nuget package. When the next release of that comes out, I will patch up the nuget references in this branch.

@clrudolphi clrudolphi self-assigned this Sep 16, 2024
@gasparnagy
Copy link
Contributor

Build is failing b/c I have a private build of Cucumber Messages nuget package. When the next release of that comes out, I will patch up the nuget references in this branch.

This is fine. We have feed https://www.myget.org/feed/Packages/reqnroll-unstable where we could publish interim releases (of dependencies as well) and I think this feed is added to the nuget sources, so the build would be able to resolve it though them. But I never tested this model yet.

…ecutionEvents to one or more Message Sinks. Default implementation of one Sink that serializes Messages to an ndjson file per feature.

Not finished: hooking up the TestThreadExecution event handlers.
…mber Messages Publisher and Broker to the initialization sequence provided by the Plugin architecture.
…est project. The File Sink is hooked in as a nuget package.

Next: exploratory debugging to confirm the event listeners are wired up correctly at Feature start time.
Need to figure out why trace logging also not working.
…check of whether it had been initialized (temp).

(Temp) - added Debugger.Launch to FileSinkPlugin to force launch of debugger.
…Publisher from the FileSinkPlugIn completely.
…the use of a BlockingCollection. Eliminated base class for Sinks. Eliminated use of Async for MessageSinks. Sinks have to ensure their own thread-safety and non-blocking behavior. May want to revisit that.
…ke Messages (meta, Source, GherkinDocument, and Pickles).
… information into Messages. First example is transforming StepBinding classes into messages of type StepDefinition.
…pport for CucumberMessages for that feature.
…ioState emit sequences of Envelopes for consumption by the Publisher.
…fter scenario completion so that we have full binding Match info available to create the TestCase message.
minimal and pending scenarios added.
… which I don't think apply to the .NET implementation.

Need to add support for images and documents as embedded resources to be added to generated test projects before attachments and hooks can be implemented and tested.
Not yet added infrastructure to compare output ndjon with expected.
…lisher, Broker, and Sink). Returned FeatureTracker back to normal, sync methods.
…at a FeatureFinishedEvent is received but not all Scenarios are yet marked as Finished, then the entire Feature success status should be set to False.
…when two Features were rewriting simultaneously.
@clrudolphi
Copy link
Contributor Author

I believe this is now ready for a shakedown and code review. I've fixed the issues that I'm aware of and completed my punch-list. Documentation is updated. I've tested it with multiple Features running in parallel and desk-checked it for thread safety for the upcoming Scenario parallel execution model.

@clrudolphi
Copy link
Contributor Author

It looks like Cucumber Messages schemas are changing in an upcoming release PR102. This will require some changes to logic within the Messages implementation.
If you can, please go ahead and provide review comments on the subsystem as it exists now; as I'm sure there is plenty to be improved.

@clrudolphi clrudolphi marked this pull request as ready for review October 24, 2024 15:36
…Enums, and TestHookStarted/Finished messages.

CCK compliance not yet available.
…ature class start-up. Modified FeatureInfo constructor to accept the FeatureMessages object as optional parameter. Had to modify a Runtime test to accomodate that change.

Also changed Generation such that we're more thoroughly emitting the 'global::' prefix on Reqnroll types (for Messages related types).
Marked the Non-Compliant CCK test scenarios as [Ignore].
…was failing now that is called well before the Messages configuration system is spun up.
…ce when throwing an 'already resolved' registration exception.
…Static Messages for a Feature were processed (which resulted in the PickleJar not being properly initialized).
Moving the FeatureTracker creation to within the double-check of the lock section (so that it is called only one time).
Updated FluentAssertion property selector rule to ignore the TestRunStartedId.
…s problems with RuntimeTests which fire events but without a full event orchestration as expected by the Publisher.
@clrudolphi
Copy link
Contributor Author

It looks like Cucumber Messages schemas are changing in an upcoming release PR102. This will require some changes to logic within the Messages implementation. If you can, please go ahead and provide review comments on the subsystem as it exists now; as I'm sure there is plenty to be improved.

Messages v27 and CCK updates have been published. Today's commits update our Messages implementation to support this release. This adds a HookType enumeration for Hooks and a TestRunStartedId attribute to the TestRunStarted and TestRunFinished messages.

… the TraceListener until the moment its needed rather than as a constructor param. This avoids a conflict when the Test Frameworks provide their own implementation (which caused an 'oibject already resolved' error in the object container). Related changes in Tests to match.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants